-
Notifications
You must be signed in to change notification settings - Fork 12.8k
fix(19577): fix regression with fully inferred types and non-null assertions #50092
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
this fix is just easier for me to understand i'll refactor the fix to fit into the repo style in a later commit
thanks to @Andarist for providing this test!
@typescript-bot pack this |
Heya @weswigham, I've started to run the perf test suite on this PR at 4a50f11. You can monitor the build here. Update: The results are in! |
Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at 4a50f11. You can monitor the build here. |
Heya @weswigham, I've started to run the diff-based user code test suite on this PR at 4a50f11. You can monitor the build here. Update: The results are in! |
Heya @weswigham, I've started to run the extended test suite on this PR at 4a50f11. You can monitor the build here. |
Heya @weswigham, I've started to run the tarball bundle task on this PR at 4a50f11. You can monitor the build here. |
@weswigham Here are the results of running the user test suite comparing Everything looks good! |
Heya @weswigham, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
Hey @weswigham, I've packed this into an installable tgz. You can install it for testing by referencing it in your
and then running There is also a playground for this build and an npm module you can use via |
@weswigham Here they are:
CompilerComparison Report - main..50092
System
Hosts
Scenarios
TSServerComparison Report - main..50092
System
Hosts
Scenarios
Developer Information: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, the tests came back clean and the change seems somewhat reasonable - prioritizing the undefined
starting type for a non-null-asserted auto type seems fair, as does getting the nonnullable type of the flow type (though I feel like the containing non-null assertion expression should maybe be doing that, but I guess you have to in order to avoid a spurious use before def error for auto-typed things).
This just needs to be updated for current main
and then DT run again as a quick double check that it's still OK.
@typescript-bot run DT |
Heya @gabritto, I've started to run the parallelized Definitely Typed test suite on this PR at d33b656. You can monitor the build here. Update: The results are in! |
Hey @gabritto, the results of running the DT tests are ready. |
@typescript-bot pack this |
Hey @gabritto, I've packed this into an installable tgz. You can install it for testing by referencing it in your
and then running There is also a playground for this build and an npm module you can use via |
Fixes #19577